Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/sql put data unicode to bytes memory leak #832

Conversation

Mizaro
Copy link
Contributor

@Mizaro Mizaro commented Oct 5, 2020

This is a fix to prevent the memory leak from Issue #802

In my investigation of the code's behavior, It seems that when there is a need to convert Unicode string to bytes, the newly created objects of bytes will never get dereference, thus staying in memory.

@gordthompson
Copy link
Collaborator

Hi @Mizaro

I believe that if you apply this patch file to your branch …

gitignore_patch.txt

… it should get rid of the merge conflict with the current master branch at

aa52358

@gordthompson
Copy link
Collaborator

@Mizaro - BTW, if you push another commit to this PR it may also fix the AppVeyor failure. I just submitted a trivial PR (#848) as a test and its AppVeyor check passed. It looks like the AppVeyor CI environment has been changed since you submitted this PR.

@gordthompson
Copy link
Collaborator

Hey @Mizaro - Any chance that you could apply the .gitignore patch in my previous comment? @mkleehammer might be more inclined to merge the PR if he doesn't have to mess around with conflicts.

(I tried to submit the change as a PR to your fork, but GitHub wouldn't let me. I could submit PRs to a whole bunch of other forks, but yours wasn't among them. I've encountered the same issue in the past, but I don't understand why it happens.)

@Mizaro
Copy link
Contributor Author

Mizaro commented Dec 2, 2020

@gordthompson I will do it! Just not today 😅

mkleehammer pushed a commit that referenced this pull request Jan 22, 2021
Updated .gitignore

Added test for the Issue

Convert to bytes to make code more readable and work

make the memory leak bigger :)

deref objCell if it was assigned to be bytes that was created from unicode cell info
@mkleehammer
Copy link
Owner

Manually squashed and rebased. Thanks!

@Mizaro
Copy link
Contributor Author

Mizaro commented Jan 22, 2021

@mkleehammer, Thanks for inserting it :)

@Mizaro
Copy link
Contributor Author

Mizaro commented Jun 25, 2021

@gordthompson Thanks for being so proactive !

@thealbres
Copy link

Hello, guys! Can you confirm if this issues has already been deployed? I am having something really similar, but my version is the newest 4.0.32

I read the Relases notes and didn't find anything related to #832

@gordthompson
Copy link
Collaborator

@thealbres - This patch was merged as 716572a on 2021-01-21 and included in version 4.0.31, released 2021-07-03.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants